fix: make SQLAlchemy import lazy and declare it under [sink] extra - #71
fix: make SQLAlchemy import lazy and declare it under [sink] extra#710x070907 wants to merge 8 commits into
Conversation
|
The issue itself is real, but this PR is now a bit out of date against main. The pyproject.toml package-data entry for state/schema.sql is already present there, so the remaining value here is really the MANIFEST.in addition rather than both changes together. The verification note also needs to be tightened. Importing init_state_db does not actually exercise the schema file read, so it does not fully prove the packaging issue is fixed. A better check would be to call init_state_db(...) against a temporary database path after a normal install and confirm that it no longer raises FileNotFoundError. |
|
Hi @0-Shimanshu, I have added the comment |
|
The main fix looks correct now. Making the SQLAlchemy import lazy inside Before merge, please do one small cleanup pass:
After that, this is good to merge. |
|
The lazy import direction is right, but I want to adjust the dependency boundary a bit before merging. SQLAlchemy sink is not really a Flask-specific feature. Django, Flask, FastAPI, or even plain Python usage could all use the sink path. So I do not think SQLAlchemy should live under a Could you update this PR to use a dedicated sink extra instead? Something like: [project.optional-dependencies]
sink = [
"sqlalchemy",
] |
|
@0-Shimanshu , sure I will make the necessary changes and update the branch and let you know |
06dd362 to
eeab3a7
Compare
|
@0-Shimanshu , I have made the necessary changes and the flask app doesn't crash anymore.
After i run
Should I update the README,quickstart.md and installation.md with |
|
The sink dependency should be documented as a dedicated extra, not under Flask. For local development, use: That works because . means the current Python project, and pip reads the package name from pyproject.toml. And later, once we publish to PyPI, users can use: Please update README / quickstart / installation docs with this wording. |
|
Sure, I'll update it |
|
@0x070907 update |
e8a0749 to
c369d62
Compare
|
Hi @0-Shimanshu, apologies for the delayed response - My exams are going on. I've now addressed all the feedback:
|
|
Should I update |
like what can u first tell me the changes. |
I will add a note about the |
i think that would be good go ahead |
…xtra for sqlalchemy
…and sqlalchemy integration
2980986 to
7ed80f0
Compare
Its done,please check |
|
@0x070907 dont force push it break stuff and u pushed i project.toml unecesseary file |
|
okay,I wont force push again.But pyproject.toml is necessary because |
oo ya right |
51fa4ff to
7ed80f0
Compare
|
@0-Shimanshu I accidentally got a merge commit on this branch,and in order to remove the commit without affecting any files or the commit history,I had to force push and I confirm that this hasn't affected any file or the main branch |


Summary
This PR makes the import lazy and declares sqlalchemy under a proper
[sink]optional extra inpyproject.tomlso dependency declarations match actual runtime expectations.Why
The Flask integration imports
_sink_modefrom sqlalchemy.py at module scope, and sqlalchemy.py importsSQLAlchemyat the top level. Since sqlalchemy was only listed under[dev], importingAdiuvareMiddlewarecrashed on a plain install(pip install .) .adiuvare/integrations/sqlalchemy.py- Removed top-levelfrom sqlalchemy import event. Moved it insideattach_sink()as a lazy import pointing atpip install adiuvare[sink]pyproject.toml- Added[sink]optional extrasqlalchemyso dependency declarations match runtime expectations.tests/test_flask.py- Addedtest_flask_middleware_import_does_not_require_sqlalchemywhich proves flask import works without sqlalchemy.tests/test_sink.py- Addedpytest.importorskipguard at top,so it gracefully skips when sqlalchemy isn't installed instead of crashingScope
Verification
Commands run:
#2. verify sink extra pulls pip install adiuvare[sink] pytest tests/test_sink.py -vNotes:
Signal-specific notes
If this PR adds or changes a signal or a meaningful detection pattern, fill this in.
Signal type:
trackA)trackB)Source, reproducer, or proof: N/A
Benign case checked: N/A
Performance note: N/A
TUI notes
If this PR changes the TUI, include before/after screenshots or a short capture.
Local-only file check
adiuvare.yamlor localconftest.pychangesRelated issue
Closes #67